Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove dependency on apollo CLI for fetching schema #1935

Merged
merged 49 commits into from
Sep 16, 2021
Merged

Remove dependency on apollo CLI for fetching schema #1935

merged 49 commits into from
Sep 16, 2021

Conversation

calvincestari
Copy link
Member

@calvincestari calvincestari commented Sep 10, 2021 •
edited
Loading

This change removes a dependency on the Apollo CLI for schema downloading. Schema downloads are now Swift-based and maintainable/extensible within apollo-ios.

Notable changes:

  • Abstracted URLDownloader from CLIDownloader; used in both CLI and schema downloading. CLI downloading will be removed along with the Swift codegen implementation.
  • Registry schema requests are done via UntypedGraphQLRequestBodyCreator - a non-type-safe request creator to facilitate sending requests not using code generation.
  • Refactored ApolloSchemaOptions
    • Renamed to ApolloSchemaDownloadConfiguration for a more specific naming context.
    • Schema output is always SDL.
    • Defaults to the current registry variant.
  • Renamed ApolloSchemaDownloader APIs
    • fetch(with:) is more expressive than run(options:).

Closes #1823

designatednerd and others added 30 commits September 10, 2021 16:19
@designatednerd @calvincestari
Pull downloader into its own file
1de9881
@designatednerd @calvincestari
Use URL downloader in CLI downloader
3610cfa
@designatednerd @calvincestari
Update headers to take both key and value
93ec2e1
@designatednerd @calvincestari
Add Apollo-Use-Only method to create a non-type safe GraphQL request
d277334
@designatednerd @calvincestari
Add methods for direct download from registry and introspection rathe…
6a7a606 
…r than using the JS CLI
@designatednerd @calvincestari
Revert "Add Apollo-Use-Only method to create a non-type safe GraphQL …
2e546a5 
…request"

This reverts commit 9aac6367a611260ffea12e3b0ae965b999d9b9d3.
@designatednerd @calvincestari
Pull untyped request body creator into codegen lib
8aa6e46 
I tried pulling some of this stuff into Core but you have to pull so much into core to make it work since by default it uses a bunch of `Apollo` only types that it kind of defeats the point of keeping core small, and realistically the only place where we even *should* be using untyped queries is in grabbing the schema.
@designatednerd @calvincestari
update types on codegen tests
b314b97
@designatednerd @calvincestari
Pull out usage of the typescript CLI 🎉
95744c6
@designatednerd @calvincestari
Make sure URL request headers are properly set
b465bbb
@designatednerd @calvincestari
don't try to import all of apollo 🤦‍♀️
153a3c2
@designatednerd @calvincestari
Make sure keys are sorted
5bff0ef
@designatednerd @calvincestari
Make sure folder is created before trying to write to it
8439a0a
@designatednerd @calvincestari
Update tests
f791135
@designatednerd @calvincestari
Update that we should really only be getting SDL files, not JSON, bec…
af8a782 
…ause the JSON is so damned huge
@designatednerd @calvincestari
start adding and testing downloading the schema from the graph registry
812d12a
@designatednerd @calvincestari
Turns out variant is actually required
e8c4faf
@designatednerd @calvincestari
fix expected file name
815a718
@designatednerd @calvincestari
get rid of arguments parameter, update custom debug strings
5c7769f
@designatednerd @calvincestari
Turns out you have to _throw_ the error rather than just creating it …
59dde6b 
…🤦‍♀️
@designatednerd @calvincestari
Check that downloaded info from introspection query can be loaded int…
2600a93 
…o an actual schema rather than just being json
@designatednerd @calvincestari
test that downloaded SDL schema from the Registry can be turned into …
9fe96ee 
…a schema
@designatednerd @calvincestari
Clarify where we're converting from a downloaded Registry file vs a d…
83c1e57 
…ownloaded Introspection file.
@designatednerd @calvincestari
first swipe at adding SDL printing for introspection
3c5e177
@calvincestari
Updated JS bundle
3c36ebc
@designatednerd @calvincestari
add tslib to the package.json so it gets pulled in if you haven't glo…
3771b2d 
…bally installed it
@designatednerd @calvincestari
actual, successful swipe at getting introspection JSON converted to SDL.
2426893
@calvincestari
Update integration tests for new schema downloader
07f440d
@calvincestari
Organize code by section
8a77f00
@calvincestari
Fix spelling mistake
6e9aee8
@hwillson hwillson removed this from the Release 1.0 milestone Sep 13, 2021
@calvincestari calvincestari force-pushed the update/direct-schema-fetch branch from 75b7278 to 6e5e1dc Compare September 15, 2021 04:36
@calvincestari calvincestari marked this pull request as ready for review September 15, 2021 04:37
@calvincestari calvincestari changed the title Update/direct schema fetch Remove dependency on apollo CLI for fetching schema Sep 15, 2021
AnthonyMDev
AnthonyMDev approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on this! So glad to have this finished, and it looks great. Just a couple of suggestions.

One question: Do you know why the bundled ApolloCodegenFrontend.bundle.js changed? Did we need to make changes to the js library?

@designatednerd
Copy link
Contributor

designatednerd commented Sep 15, 2021 •
edited
Loading

Do you know why the bundled ApolloCodegenFrontend.bundle.js changed? Did we need to make changes to the js library?

Yeah there was one method I had to expose for moving between JSON and SDL - I'm not quite seeing where the actual code went, though. Not sure if it got eaten in a merge or what

@calvincestari
Copy link
Member Author

Do you know why the bundled ApolloCodegenFrontend.bundle.js changed? Did we need to make changes to the js library?

Yeah there was one method I had to expose for moving between JSON and SDL - I'm not quite seeing where the actual code went, though. Not sure if it got eaten in a merge or what

This is the exact diff - e.printSchema=function e(t){return e()},. Goes together with this and this.

AnthonyMDev
AnthonyMDev approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

designatednerd
designatednerd approved these changes Sep 16, 2021
View reviewed changes
Copy link
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question re testing, otherwise this is :shipit:

@calvincestari calvincestari merged commit 22e2097 into main Sep 16, 2021
@calvincestari calvincestari deleted the update/direct-schema-fetch branch September 16, 2021 04:18
@hwillson hwillson added this to the Release 0.49.0 milestone Sep 16, 2021
@calvincestari calvincestari mentioned this pull request Sep 20, 2021
Merged
13 tasks
@calvincestari calvincestari mentioned this pull request Feb 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@designatednerd designatednerd designatednerd approved these changes

@AnthonyMDev AnthonyMDev AnthonyMDev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 0.49.0
Development

Successfully merging this pull request may close these issues.

Remove dependency on apollo CLI for fetching schema
4 participants
@calvincestari @designatednerd @AnthonyMDev @hwillson